Skip to content

refactor: modernize type hints and enable ruff UP#12

Merged
gregsvo merged 1 commit into
ActiveCampaign:mainfrom
purificant:refactor/modernize-type-hints
Jul 7, 2026
Merged

refactor: modernize type hints and enable ruff UP#12
gregsvo merged 1 commit into
ActiveCampaign:mainfrom
purificant:refactor/modernize-type-hints

Conversation

@purificant

Copy link
Copy Markdown
Contributor

This PR updates type hints to use modern python syntax for 3.10+ released in October 2021.

It can be reproduced with the following three actions:

  1. Add target-version = "py310" to pyproject.toml which explicitly tells ruff what the minimum supported python version is.
  2. Expand the ruff ruleset to include the UP rules.
  3. Run
ruff check postmark --fix
ruff format postmark

@gregsvo gregsvo left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The `builtins.list` usage in `webhooks/manager.py` and `outbound/manager.py` is a ruff UP autofix quirk: when a class defines a method named `list`, ruff qualifies the builtin to avoid shadowing. The underlying concern is real in `webhooks/manager.py` (no `from future import annotations`, and the `list` method appears before `create`/`edit`), but the fix of choice should be `from future import annotations` rather than `builtins.list`. `outbound/manager.py` already has that import, so `builtins.list` there is dead weight.

Suggest:

  • Add `from future import annotations` to `webhooks/manager.py`
  • Drop `import builtins` from both files
  • Use plain `list` everywhere

@gregsvo gregsvo merged commit e4e3256 into ActiveCampaign:main Jul 7, 2026
6 checks passed
@purificant

Copy link
Copy Markdown
Contributor Author

ruff does it this way so the annotations don't depend on where the list method is defined. Without builtins, any annotation using the list type has to come before async def list.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants